📖 Update InfraCluster provider contract to account for the paused condition #10519

theobarberbany · 2024-04-25T15:45:37Z

What this PR does / why we need it:
This change updates the infra cluster provider contract to account for a new paused
condition.

Relates #10130

/area documentation

k8s-ci-robot · 2024-04-25T15:45:47Z

Hi @theobarberbany. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

theobarberbany · 2024-04-26T10:40:51Z

/area documentation

docs/book/src/developer/architecture/controllers/control-plane.md

sbueringer · 2024-04-29T15:36:14Z

/ok-to-test

docs/book/src/developer/architecture/controllers/control-plane.md

fabriziopandini · 2024-09-26T10:43:58Z

@theobarberbany things are moving fast in the CAPI world!

FYI, https://github.com/fabriziopandini/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md recently merged, introducing a paused conditions in all CAPI resources.

Also as a side effect, there is a major refactor of docs in progress on main WRT to provider contract pages (I also gave an update yesterday at the office hours). Work is tracked in #11105 (a few PR are still missing, but I hope to complete this work by end of next week)

It would be great if contract changes in this PR can build on top of it 😅

theobarberbany · 2024-09-26T11:12:33Z

Hey @fabriziopandini,

That's great! I was about to ask, as the Infra Cluster contract looks significantly different to the others!

I'll try and build off of that, as it seems the others will follow it's style (e.g https://github.com/kubernetes-sigs/cluster-api/pull/11223/files) How does that sound? :D

Also is there anything else in #11105 I can do to help?

theobarberbany · 2024-09-26T14:38:41Z

@fabriziopandini I've updated this PR to add the paused condition to the InfraCluster provider contract. Can you let me know if these changes look reasonable? :)

docs/book/src/developer/providers/contracts/infra-cluster.md

fabriziopandini · 2024-09-30T12:48:58Z

docs/book/src/developer/providers/contracts/infra-cluster.md

@@ -308,6 +308,8 @@ If a condition with type `Ready` exist, such condition will be mirrored in Clust
 Please note that the `Ready` condition is expected to surface the status of the InfraCluster during its own entire lifecycle,
 including initial provisioning, the final deletion process, and the period in between these two moments.

+Provider implementers SHOULD implement `Status.Conditions[Paused]` to report if the cluster is paused. It should check if 'spec.paused' is set on the cluster, and for the paused annotation. Implementing this condition is currently optional, but we may revisit this in the future.


I think I would add a dedicated, optional rule for pause stating that

providers SHOULD implement the pause behaviour for every object with a reconciliation loop (eg. pause machines when the cluster or the object is paused; pause does not makes sense for templates)

if implementing the pause behaviour, providers SHOULD surface the pause status for an object with a condition

Also, let's be specific and name the annotation that trigger pause at object level

Ok, I think I've updated as you've asked! PTAL :)

docs/book/src/developer/providers/contracts/infra-cluster.md

fabriziopandini · 2024-10-07T09:54:26Z

docs/book/src/developer/providers/contracts/infra-cluster.md

@@ -335,6 +335,12 @@ the implication of this choice which are described both in the document above an

 </aside>

+### InfraCluster: Pausing


Please link this rule from the table up at L44 on this page (you can add this as a new line at the end)

I've added at the end, not sure if it should be underneath the other InfraCluster: <foo> lines though?

fabriziopandini · 2024-10-08T08:48:03Z

/lgtm
cc @sbueringer @chrischdi for a final check on the contract change (same will be applied to other contracts I guess)

k8s-ci-robot · 2024-10-08T08:48:09Z

LGTM label has been added.

Git tree hash: 43f75dcf90b7bc9e4b5722c10e2557495332b536

sbueringer · 2024-10-10T17:59:06Z

Thx!

/approve

k8s-ci-robot · 2024-10-10T17:59:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 25, 2024

k8s-ci-robot requested review from elmiko and jackfrancis April 25, 2024 15:45

k8s-ci-robot added do-not-merge/needs-area PR is missing an area label needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 25, 2024

k8s-ci-robot added area/documentation Issues or PRs related to documentation and removed do-not-merge/needs-area PR is missing an area label labels Apr 26, 2024

theobarberbany force-pushed the pause-condition-update-contract branch from 285b232 to 2ff8fbd Compare April 26, 2024 11:59

theobarberbany changed the title ~~WIP: Update provider contract to account for the paused condition~~ Update provider contract to account for the paused condition Apr 26, 2024

theobarberbany marked this pull request as ready for review April 26, 2024 12:18

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 26, 2024

k8s-ci-robot requested review from richardcase and stmcginnis April 26, 2024 12:18

theobarberbany mentioned this pull request Apr 26, 2024

Consistent reporting of paused resources #10130

Open

JoelSpeed reviewed Apr 29, 2024

View reviewed changes

docs/book/src/developer/architecture/controllers/control-plane.md Outdated Show resolved Hide resolved

sbueringer changed the title ~~Update provider contract to account for the paused condition~~ 📖 Update provider contract to account for the paused condition Apr 29, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 29, 2024

sbueringer reviewed May 2, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2024

theobarberbany force-pushed the pause-condition-update-contract branch from 2ff8fbd to 24a20d0 Compare July 8, 2024 11:02

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 8, 2024

theobarberbany force-pushed the pause-condition-update-contract branch 3 times, most recently from 7207fac to 4d6f9c2 Compare July 8, 2024 13:15

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 25, 2024

theobarberbany force-pushed the pause-condition-update-contract branch 2 times, most recently from 5be71d8 to 965cb82 Compare September 25, 2024 17:59

theobarberbany force-pushed the pause-condition-update-contract branch from 965cb82 to b4e6936 Compare September 26, 2024 14:36

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 26, 2024

theobarberbany changed the title ~~📖 Update provider contract to account for the paused condition~~ 📖 Update InfraCluster provider contract to account for the paused condition Sep 26, 2024

theobarberbany requested review from sbueringer and fabriziopandini September 27, 2024 11:34

JoelSpeed reviewed Sep 27, 2024

View reviewed changes

docs/book/src/developer/providers/contracts/infra-cluster.md Outdated Show resolved Hide resolved

theobarberbany force-pushed the pause-condition-update-contract branch from b4e6936 to adfa182 Compare September 27, 2024 13:40

fabriziopandini reviewed Sep 30, 2024

View reviewed changes

theobarberbany force-pushed the pause-condition-update-contract branch from adfa182 to ccd3d15 Compare October 1, 2024 14:13

fabriziopandini reviewed Oct 7, 2024

View reviewed changes

Update InfraCluster contract to include Pausing rule

c6a8027

theobarberbany force-pushed the pause-condition-update-contract branch from ccd3d15 to c6a8027 Compare October 7, 2024 10:53

k8s-ci-robot assigned fabriziopandini Oct 8, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 8, 2024

theobarberbany mentioned this pull request Oct 8, 2024

📖 Update InfraMachine contract to include pausing rule #11275

Merged

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 10, 2024

k8s-ci-robot merged commit fb691b8 into kubernetes-sigs:main Oct 10, 2024
16 checks passed

k8s-ci-robot added this to the v1.9 milestone Oct 10, 2024

nojnhuh mentioned this pull request Oct 11, 2024

Set Paused condition to align with new CAPI recommendation kubernetes-sigs/cluster-api-provider-azure#5171

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

📖 Update InfraCluster provider contract to account for the paused condition #10519

📖 Update InfraCluster provider contract to account for the paused condition #10519

theobarberbany commented Apr 25, 2024 •

edited

Loading

k8s-ci-robot commented Apr 25, 2024

theobarberbany commented Apr 26, 2024

sbueringer commented Apr 29, 2024

fabriziopandini commented Sep 26, 2024

theobarberbany commented Sep 26, 2024

theobarberbany commented Sep 26, 2024

fabriziopandini Sep 30, 2024

theobarberbany Oct 1, 2024

fabriziopandini Oct 7, 2024

theobarberbany Oct 7, 2024

fabriziopandini commented Oct 8, 2024

k8s-ci-robot commented Oct 8, 2024

sbueringer commented Oct 10, 2024

k8s-ci-robot commented Oct 10, 2024

		@@ -335,6 +335,12 @@ the implication of this choice which are described both in the document above an

		</aside>

		### InfraCluster: Pausing

📖 Update InfraCluster provider contract to account for the paused condition #10519

📖 Update InfraCluster provider contract to account for the paused condition #10519

Conversation

theobarberbany commented Apr 25, 2024 • edited Loading

k8s-ci-robot commented Apr 25, 2024

theobarberbany commented Apr 26, 2024

sbueringer commented Apr 29, 2024

fabriziopandini commented Sep 26, 2024

theobarberbany commented Sep 26, 2024

theobarberbany commented Sep 26, 2024

fabriziopandini Sep 30, 2024

Choose a reason for hiding this comment

theobarberbany Oct 1, 2024

Choose a reason for hiding this comment

fabriziopandini Oct 7, 2024

Choose a reason for hiding this comment

theobarberbany Oct 7, 2024

Choose a reason for hiding this comment

fabriziopandini commented Oct 8, 2024

k8s-ci-robot commented Oct 8, 2024

sbueringer commented Oct 10, 2024

k8s-ci-robot commented Oct 10, 2024

theobarberbany commented Apr 25, 2024 •

edited

Loading